Skip to content

fix: include created_at and expires_at in SmallOrder conversion#127

Merged
grunch merged 1 commit intoMostroP2P:mainfrom
Catrya:real-data-created-expires-at
Dec 24, 2025
Merged

fix: include created_at and expires_at in SmallOrder conversion#127
grunch merged 1 commit intoMostroP2P:mainfrom
Catrya:real-data-created-expires-at

Conversation

@Catrya
Copy link
Member

@Catrya Catrya commented Dec 22, 2025

fix #126
Include created_at and expires_at when converting Order to SmallOrder to ensure makers receive complete order information in direct messages when orders are republished.

Summary by CodeRabbit

  • Bug Fixes
    • SmallOrder objects now correctly populate creation and expiration timestamp information when derived from Order objects, ensuring data consistency and complete information preservation during conversions. This improves data accuracy while maintaining backward compatibility.

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 22, 2025

Walkthrough

The From<Order> conversion for SmallOrder in src/order.rs now populates created_at and expires_at fields with values from the source Order instead of defaulting to None, ensuring temporal metadata is preserved during conversion.

Changes

Cohort / File(s) Summary
SmallOrder conversion logic
src/order.rs
Modified From<Order> implementation to populate created_at and expires_at fields with Some(order.created_at) and Some(order.expires_at) respectively, instead of leaving them as None

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

A rabbit hops through orders with glee, 🐰
No more null times in republished decree,
Created and expired now travel along,
Temporal data stays where it belongs,
Tick-tock, the fix makes timing strong! ⏰

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: including created_at and expires_at fields in the SmallOrder conversion, which directly addresses the PR's primary objective.
Linked Issues check ✅ Passed The code change directly implements the requirement from issue #126 by populating created_at and expires_at in SmallOrder conversion, ensuring clients receive accurate time data for republished orders.
Out of Scope Changes check ✅ Passed All changes are scoped to the specific conversion function and directly address the linked issue; no unrelated modifications or scope creep detected.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
src/order.rs (1)

562-563: Fix properly preserves temporal data during Order to SmallOrder conversion.

The change correctly wraps created_at and expires_at in Some() when converting from Order to SmallOrder. This preserves the timestamp values for clients to calculate remaining order time, particularly when orders are republished in pending status. The approach aligns with the existing as_new_order() method and maintains backward compatibility since SmallOrder fields are already Option<i64> types.

Consider adding a regression test to verify timestamp preservation during this conversion:

#[test]
fn test_order_to_small_order_preserves_timestamps() {
    let created_at = 1609459200;
    let expires_at = 1609545600;
    
    let order = Order {
        created_at,
        expires_at,
        ..Default::default()
    };
    
    let small_order: SmallOrder = order.into();
    
    assert_eq!(small_order.created_at, Some(created_at));
    assert_eq!(small_order.expires_at, Some(expires_at));
}
📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5e83932 and 3bc86d7.

📒 Files selected for processing (1)
  • src/order.rs
🧰 Additional context used
📓 Path-based instructions (2)
src/{order,message,user}.rs

📄 CodeRabbit inference engine (AGENTS.md)

Place core domain models and logic in src/order.rs, src/message.rs, and src/user.rs

Files:

  • src/order.rs
**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Use snake_case for functions and modules
Use PascalCase for types and enums
Use SCREAMING_SNAKE_CASE for constants
Document public APIs with /// comments
Keep error enums exhaustive
Avoid leaking internals; prefer exposing a curated surface via the prelude
Place unit tests inline beside modules using #[cfg(test)] mod tests
Name tests descriptively (e.g., test_signature_roundtrip)
Cover failure paths for invalid currencies, dispute resolution, and crypto edge cases
Reuse existing builders/helpers instead of duplicating fixtures
Run cargo fmt and address clippy warnings (clippy -D warnings) before pushing

Files:

  • src/order.rs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: test

Copy link
Member

@grunch grunch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@grunch grunch merged commit 4d3d07c into MostroP2P:main Dec 24, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

When an order is republished in pending status, Mostrod sends null created_at and expires_at.

2 participants